Skip config if not in interactive mode #180
Conversation
Not sure if this is the correct way to configure it
kidonng
changed the title
|
Great idea! I will merge this as soon as I figure out how to make it still possible to run tests locally. B/c right tests only work on CI but I run them locally when I'm developing. |
|
Why not just pretend you are a CI ( Or maybe |
|
Sure that'll work for me. |
|
|
PatrickF1
changed the title
|
@kidonng can you help me answer these questions
|
Subshell here is literally just shell as a subprocess. You can call it subprocess if you don't like it, but session is something entirely different.
I'm not sure what you mean or what are you referring to, and honestly I think they are out of scope for this PR. |
I'm trying to learn these aspects of fish so I can properly write a summary of how this affects users' shell performance. |
|
What is fish session then? |
|
Ah funny that #181 was created the same day. I realized another reason this change is good: it makes fzf previews load a bit faster! |
🤔 Perhaps I should just give a short reply to those questions:
That depends on what do you mean by "command substitution". But from my view, you don't usually start another fish in command substitution so no effects.
Again, are you referring to fzf.fish, tide or fish in general? I don't think I can give a better answer than reading the code/docs directly. |
From starting a fish shell to exit this particular shell. In fact that's not a fish terminology, not even a shell terminology, it's just session's literal meaning. |
Hmm I don't think that issue has anything to do with this change. Glad you are happy about it though. |
|
Thanks for answering my questions.
It does doesn't it? When fzf loads a previews, it executes a new fish shell, which means it has to load the conf.d/fzf.fish in the process. |
That issue #181 has nothing to do with preview/shell launch speed. The faster preview has been mentioned from the very beginning in this PR's description, until it is edited:
|
😅 Oops my bad. I think I was not mentally all there when I edited it. Sorry ! |
This fixes #183, which is a resulting bug from #180. ## Explanation of the issue #180 makes `$_fzf_search_vars_command` unavailable for non-interactive sessions. However, `fzf_configure_bindings` depends on that variable being available. If the user puts `fzf_configure_bindings` in their `config.fish`, `fzf_configure_bindings` gets executed in non-interactive sessions e.g. fzf preview windows or by tide, and the following line in the function... https://github.com/PatrickF1/fzf.fish/blob/17d54b576919ee77644779db2dcae56d372a8830/functions/fzf_configure_bindings.fish#L33 ...essentially becomes: ```fish test -n $key_sequences[5] && bind --mode $mode $key_sequences[5] ``` which results in error message like `bind --preset \cv fish_clipboard_paste` or `bind: No binding found for sequence '\cv'`. ## Solution Don't execute the entirety of `fzf_configure_bindings` if not in interactive mode. And for good measure, quote `_fzf_search_vars_command` so that even if it does get executed, the bind command will run successfully.
All of fzf.fish's features are only relevant and helpful in interactive mode. However, its config gets sourced even in non-interactive mode. So, let's skip configuration in non-interactive fish shells to make them a bit faster. This greatly benefits other plugins as well; for example tide makes use of non-interactive fish shells extensively.